Skip to content

Add include directories consistently PRIVATE instead of PUBLIC#201

Closed
cwecht wants to merge 1 commit into
ros2:rollingfrom
cwecht:target_include_private
Closed

Add include directories consistently PRIVATE instead of PUBLIC#201
cwecht wants to merge 1 commit into
ros2:rollingfrom
cwecht:target_include_private

Conversation

@cwecht

@cwecht cwecht commented Jan 17, 2024

Copy link
Copy Markdown

This helps in situations in which:

  1. If the numpy include directory is determined by executing python: and
  2. If the path to your numpy installation may change.

This can happen e.g. if you want to cross-compile ROS2 and want to do it on some CICD setups.

The issue is that right now, e.g. for std_msgs export_std_msgs__rosidl_generator_pyExport.cmake sets the INTERFACE_INCLUDE_DIRECTORIES property of std_msgs::std_msgs__rosidl_generator_py to an absolute path (which causes issues if the folder of your numpy installation changes.

With this patch all include directories are consistently added PRIVATE instead of PUBLIC. There shouldn't be a need to expose them anyways.

Signed-off-by: Christopher Wecht <cwecht@mailbox.org>

@clalancette clalancette left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to ask to hold off on this one. This change is also done by the much larger #140, but that one has additional benefits. I'm just waiting for @sloretz to finish reviewing that one.

@clalancette

Copy link
Copy Markdown
Contributor

I've now merged in #140, which also contains this fix. So I'll close this one out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants